✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	2203abe
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/6764a58194b5310008d31958
😎 Deploy Preview	https://deploy-preview-2920--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Thanks @ghthor for your work here, I saw you linked to #2894, which is providing support for dynamically adding listeners to kafka. IIUC this PR is adding fixed listeners to it. What are the implications of having one or the other worlds for the kafka module here? In other words: if we merge this one, do we need the other one?

@eddumelendez could you take a look at this PR too?

@mdelapenya I think that most of the usecases for dynamic listeners would be provided by this one. The case that wouldn't be covered by this PR is if you need listeners that are a different protocol other than PLAINTEXT, IE TLS or whatever else kafka supports.

There is no reason that adding more default fixed listeners could not co-exist with the feature to add dynamic listeners. I don't know that I've found a reason to use dynamic listeners yet, and they add many elements of complication, as to declare them you need to know much information upfront or configure more stuff to make them work (hostnames / ports) etc.

The aim of this PR is to avoid as much boilerplate as possible to achieve Container <> Container support. I gave the user 2 options to achieve this aim.

1. Configure the non-kafka containers with an extra hostname host.docker.internal and use BrokersByHostDockerInternal
2. Configure both kafka and non-kafka containers to run in a new docker network so container name/id name resolution works and use BrokersByContainer{Name,Id}

You do either of those and you get Container <> Container kafka working AND we can assert all this is working within the kafka module tests because we're not dealing with user configurable listeners.

I don't know that I've found a reason to use dynamic listeners yet, and they add many elements of complication, as to declare them you need to know much information upfront or configure more stuff to make them work (hostnames / ports) etc.

A fan of this! Thanks for the explanation

We'll start reviewing this PR today, and will let @eddumelendez take a look for more kafka-specific questions (I'm totally illiterate on it)

Will push up a fix for the lint failures. 🏁

@mdelapenya can you authorize another run of the CI test suite?

@ghthor could you run make lint from the kafka directory? It will give you more context about the lint error.

  modules/kafka/examples_test.go:12: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/testcontainers) (gci)
  	"github.com/docker/docker/api/types/container"
  Error: modules/kafka/examples_test.go:73:4: exitAfterDefer: log.Fatalf will exit, and `defer func(){...}(...)` will not run (gocritic)
  			log.Fatalf("failed to get container state: %s", err)
  			^
  Error: modules/kafka/examples_test.go:182:4: exitAfterDefer: log.Fatalf will exit, and `defer func(){...}(...)` will not run (gocritic)
  			log.Fatalf("failed to get container state: %s", err)
  			^
  Error: modules/kafka/examples_test.go:284:4: exitAfterDefer: log.Fatalf will exit, and `defer func(){...}(...)` will not run (gocritic)
  			log.Fatalf("failed to get container state: %s", err)
  			^
  modules/kafka/kafka.go:66: File is not `gofumpt`-ed (gofumpt)

@mdelapenya Done

@eddumelendez what are your thoughts on this PR?

The proposed idea is good; however, I lean towards prioritizing user flexibility. Introducing these changes as defaults could potentially break existing users who have already defined their own listeners.

Do you have an example of how to do that, as this change is proposed because that's not really possible to do.

Additionally, there is a specific use case that this approach does not address: scenarios where tools like Toxiproxy are used in front of Kafka to simulate network conditions and test resiliency.

Do you have an example of what this looks like. I don't understand how adding a few more listeners and advertised listeners would break this.

@eddumelendez

I could see what it looks like to put all this behind functional options. It would differ from the other PR in that we're not going to allow you to configure any of them, just whether they are enabled or not.

This doesn't have a benefit that we could assert some things and produce errors if the API isn't used correctly.

Example being, if you want to use the BrokersByContainer{Name,Id} API you need to run the testcontainer in a docker network, not the default one. So if you enable that and don't pass in a network then we can produce a helpful error.

Using the BrokersByHostDockerInternal couldn't be verifed in the same way, as that requires configuring the OTHER container, But the other 2 methods could verify that; and actually we should probably produce an error for those 2 now cause we know whether kafka was started in the default network or not.

I have reviewed various use cases and examined bugs reported in other language implementations. Below is a summary of the key requirements we should address in the API:

1. Avoid Using host.docker.internal as a default
   Using host.docker.internal as a default can create confusion with the equivalent feature provided by Docker Desktop (reference). Moreover, it required additional configuration in the companion container, making it less convenient for users.

2. Flexible listener Configuration
   Listeners can be configured using networking to be accessed via container name or container ID, as demonstrated in this PR. However, some users may prefer to utilize the container’s IP address. The API should accommodate these varied preferences, providing users with flexibility in how they configure and use it.

3. Proxy Configuration for Kafka Broker
   As previously mentioned, a proxy can be configured on top of the Kafka broker. For this to function effectively, the proxy will need to register the network alias or the container IP to facilitate communication between containers.